Skip to content

Comments

Graded pr#19

Draft
spritezs wants to merge 24 commits intodevfrom
gradedPr
Draft

Graded pr#19
spritezs wants to merge 24 commits intodevfrom
gradedPr

Conversation

@spritezs
Copy link

Created new branch and removed heavy files from pr. Only left install-savilerow.sh in the bin dir since that's the only one I ve changed.

spritezs and others added 24 commits October 15, 2024 16:28
… to specify the problem directory path containing the generator, model, and repair, rather than specifying each individual path. Right now it only works with essence problems but I can easily make it work with mzn problems as well. Only rule is that the problem needs to be called problem.essence, the generator generator.essence, and repair file repair.essence
This reverts commit 3aff897.
…nly need to specify the problem directory path containing the generator, model, and repair, rather than specifying each individual path. Right now it only works with essence problems but I can easily make it work with mzn problems as well. Only rule is that the problem needs to be called problem.essence, the generator generator.essence, and repair file repair.essence"

This reverts commit 9328e76.
@spritezs spritezs requested a review from ndangtt November 29, 2024 11:33
@ndangtt
Copy link
Contributor

ndangtt commented Nov 29, 2024

@spritezs: thank you for the PR. There are a number of issues that I think we'd need to address before merging:

  • bin: the content of the folder should be similar to the one in the dev and main branch: the installation scripts shouldn't be removed. The ones that should be removed, as mentioned in my previous comments, was the installation folders, as those were created automatically when we run the installation scripts.
  • it'd be best to remove those heavy files completely from github's history. At the moment they were still in the new branch, and there was a commit that removed them.
  • https://github.com/stacs-cp/AutoIG/tree/gradedPr/data/models/car_problem_only: I suppose this folder was for testing conjure functionality. This should be included in a different PR. It'd be best to keep the changes to minimal and only include the ones specific to this PR's functionality.
  • The commit history of here include things that aren't related to this PR, it'd be nice if we could avoid those.

I had a discussion with Oz last Wednesday, and it seems that the best solution is to make a fresh fork of the current dev branch on your own github account, do the changes that related to this functionality, and create a PR with that fork. It'd be helpful if you could summary the list of changes when submitting the new PR too. Could you please help try that again? Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants